[clang][AST] Pretty-print default template template args #162134
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Andrey Ali Khan Bolshakov (bolshakov-a) ChangesFull diff: https://github.com/llvm/llvm-project/pull/162134.diff 4 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 05379f44a0a39..971af32efa85c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -124,7 +124,7 @@ AST Dumping Potentially Breaking Changes
``__atomic_test_and_set(p, 0)``
- Pretty-printing of templates with inherited (i.e. specified in a previous
- redeclaration) default arguments has been fixed.
+ redeclaration) and template default arguments has been fixed.
Clang Frontend Potentially Breaking Changes
-------------------------------------------
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 7001adeff5397..b2a837b82bb13 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -1190,7 +1190,11 @@ void DeclPrinter::printTemplateParameters(const TemplateParameterList *Params,
VisitNonTypeTemplateParmDecl(NTTP);
} else if (auto TTPD = dyn_cast<TemplateTemplateParmDecl>(Param)) {
VisitTemplateDecl(TTPD);
- // FIXME: print the default argument, if present.
+ if (TTPD->hasDefaultArgument() && !TTPD->defaultArgumentWasInherited()) {
+ Out << " = ";
+ TTPD->getDefaultArgument().getArgument().print(Policy, Out,
+ /*IncludeType=*/false);
+ }
}
}
diff --git a/clang/test/AST/ast-print-record-decl.c b/clang/test/AST/ast-print-record-decl.c
index fd815881ebeb0..394f837c3645d 100644
--- a/clang/test/AST/ast-print-record-decl.c
+++ b/clang/test/AST/ast-print-record-decl.c
@@ -315,4 +315,11 @@ template <int, int = 0> KW SmearedNTTPDefArgs;
// PRINT-CXX-NEXT: template <int = 0, int> [[KW]] SmearedNTTPDefArgs;
template <int = 0, int> KW SmearedNTTPDefArgs;
+// PRINT-CXX-LABEL: Tpl
+template <int> KW Tpl;
+// PRINT-CXX-NEXT: template <template <int> class, template <int> class = Tpl> [[KW]] SmearedTplDefArgs;
+template <template <int> class, template <int> class = Tpl> KW SmearedTplDefArgs;
+// PRINT-CXX-NEXT: template <template <int> class = Tpl, template <int> class> [[KW]] SmearedTplDefArgs;
+template <template <int> class = Tpl, template <int> class> KW SmearedTplDefArgs;
+
#endif
diff --git a/clang/unittests/AST/DeclPrinterTest.cpp b/clang/unittests/AST/DeclPrinterTest.cpp
index 28750c41796d4..2d415c46f15ac 100644
--- a/clang/unittests/AST/DeclPrinterTest.cpp
+++ b/clang/unittests/AST/DeclPrinterTest.cpp
@@ -1090,7 +1090,7 @@ TEST(DeclPrinter, TestClassTemplateDecl9) {
"template<typename T> struct Z { };"
"template<template<typename U> class T = Z> struct A { };",
classTemplateDecl(hasName("A")).bind("id"),
- "template <template <typename U> class T> struct A {}"));
+ "template <template <typename U> class T = Z> struct A {}"));
}
TEST(DeclPrinter, TestClassTemplateDecl10) {
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
@erichkeane, @cor3ntin, @mizvekov
The changes suggested by formatter would cause the TestClassTemplateDecl9 test case to stand out from the neighboring test cases, which use 4-symbol indentation. Should I apply them or put // clang-format off?
clang/docs/ReleaseNotes.rst
Outdated
|
|
||
| - Pretty-printing of templates with inherited (i.e. specified in a previous | ||
| redeclaration) default arguments has been fixed. | ||
| redeclaration) and template default arguments has been fixed. |
There was a problem hiding this comment.
Not sure if this is clear and correct.
There was a problem hiding this comment.
I'd advise to add a separate entry to the change log, and say something to the effect you have fixed pretty printing of default arguments to template template parameters.
clang/lib/AST/DeclPrinter.cpp
Outdated
| Out << " = "; | ||
| TTPD->getDefaultArgument().getArgument().print(Policy, Out, | ||
| /*IncludeType=*/false); | ||
| } |
There was a problem hiding this comment.
Default argument handling is similar for all the three kinds. Maybe, it should be a template method VisitTemplateDefArg? Or it is not justified here?
Moreover, maybe, VisitTemplateDecl call and the default argument handling should be united into a VisitTemplateTemplateParmDecl to be similar with the previous two branches?
There was a problem hiding this comment.
Yeah the other cases are in pretty much the same situation, and they are also only used here.
Either moving all of them inline, or moving this one to a likewise separate function seems fine to me.
There was a problem hiding this comment.
And what about
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -51,6 +51,15 @@ namespace {
void PrintObjCTypeParams(ObjCTypeParamList *Params);
void PrintOpenACCRoutineOnLambda(Decl *D);
+ template <typename T>
+ void VisitTemplateDefArg(const T* Arg) {
+ if (Arg->hasDefaultArgument() && !Arg->defaultArgumentWasInherited()) {
+ Out << " = ";
+ Arg->getDefaultArgument().getArgument().print(Policy, Out,
+ /*IncludeType=*/false);
+ }
+ }
+
public:?
There was a problem hiding this comment.
I've added VisitTemplateTemplateParmDecl method.
You can clang-format the affected region in a separate NFC patch and commit that directly. |
Only TestClassTemplateDecl* tests have been formatted because TestClassTemplateDecl9 is affected in llvm#162134, and because clang-format makes the code worse for some other test cases.
Only `TestClassTemplateDecl*` tests have been formatted because `TestClassTemplateDecl9` is affected in #162134, and because clang-format makes the code worse for some other test cases.
bolshakov-a
force-pushed
the
def_tpl_tpl_arg_print
branch
from
550e585 to
6afe3fe
Compare
|
Rebased. |
|
Could someone merge it? |
No description provided.